Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #460 - missing parse_rule method #463

Merged
merged 3 commits into from
Sep 13, 2022

Conversation

plowman
Copy link
Contributor

@plowman plowman commented Jul 28, 2022

What's this all about?

This is an attempt to fix issue #460, where werkzeug used to expose a function werkzeug.routing.parse_rule but made it private as of werkzeug==2.2.0.

The method was extracted from this change which seems to be when it was fully removed from werkzeug.

Copying it into this project because the method is self-contained. One alternative would be to pin the werkzeug version, which isn't a terrible idea, but also means this project would miss out on future werkzeug improvements. 🤷 Neither option is great but copy-paste is simple so that's what I did.

What's next before this can merge?

  • ❌ Right now the ossaudit check is failing, which prevents the other checks from running. It is not clear to me how to fix the failing tests. For example, it seems to be complaining that there is vulnerability in the latest version of flask. Which is good to know but unfixable:
+------------+---------+----------------------------------------------------+
|    name    | version |                       title                        |
+============+=========+====================================================+
| setuptools | 56.0.0  | 1 vulnerability found                              |
+------------+---------+----------------------------------------------------+
| pip        | 22.2.1  | [CVE-20[18](https://github.com/python-restx/flask-restx/runs/7569430787?check_suite_focus=true#step:5:19)-20225] CWE-20: Improper Input Validation |
+------------+---------+----------------------------------------------------+
| Flask      | 2.1.3   | 1 vulnerability found                              |
+------------+---------+----------------------------------------------------+
| dparse     | 0.5.1   | 1 vulnerability found                              |
+------------+---------+----------------------------------------------------+
Found 4 vulnerabilities in 73 packages
  • ✅ The black syntax check is passing when I run locally.
  • ❌ Tests are failing. There appears to be another, unrelated issue from werkzeug which broke the tests.

This method was removed from the public API in werkzeug 2.2.0. Copying it into this project because the method is self-contained.
@plowman plowman marked this pull request as ready for review July 28, 2022 22:17
flask_restx/swagger.py Outdated Show resolved Hide resolved
@ptmcg
Copy link

ptmcg commented Aug 2, 2022

@plowman Can you try rerunning the checks (which should run automatically if you update the docstring as suggested by @mmaylat )? Flask 2.2.0 has dropped, which may address the Flask vuln check. Not sure what can be done about the others though.

@ptmcg
Copy link

ptmcg commented Aug 2, 2022

Fixing the dparse vuln may be more difficult, as that project was notified of a security vuln on 31 Aug 2021, with no visible follow-up action.

@ptmcg ptmcg mentioned this pull request Aug 2, 2022
@davidism
Copy link

davidism commented Aug 2, 2022

@ptmcg What CVE is it referring to? I'm not aware of any valid CVEs for Flask 2.1. I've given up on the CVE process because it's completely out of my control what random people report to it, and I haven't had success removing invalid ones in the past.

@ptmcg
Copy link

ptmcg commented Aug 2, 2022

@davidism The CVE was in dparse (a dependency of flask-rest), not flask-rest itself, and the maintainers of that package have posted that there will be a fix in the next few days.

But I have no idea how one will address vulns in setuptools (a more recent version exists, 63.3.0, so maybe it is fixed with an update) or pip (latest version is 22.2.1, so... ?).

@davidism
Copy link

davidism commented Aug 2, 2022

You and the OP refer to a Flask vulnerability here. And I'd be very surprised if setuptools or pip are relevant. That pasted report isn't really helpful, it doesn't list what any of the vulnerabilities are, but the pip one says "improper input validation" which I struggle to see as an issue affecting the use of this library.

@ptmcg
Copy link

ptmcg commented Aug 2, 2022

You and the OP refer to a Flask vulnerability here.

I was referring to the ossaudit which includes this line:

| Flask      | 2.1.3   | 1 vulnerability found                              |

I know nothing beyond that, except that whatever it is, I suggested that they should re-run the check in the face of an updated Flask version.

If ossaudit is not valid (raising unresolvable false positives), then that will probably require a different discussion with the maintainers of this project.

@plowman
Copy link
Contributor Author

plowman commented Aug 2, 2022

@ptmcg wrote:

@plowman Can you try rerunning the checks (which should run automatically if you update the docstring as suggested by @mmaylat )? Flask 2.2.0 has dropped, which may address the Flask vuln check. Not sure what can be done about the others though.

Okay, I ran again and it's still failing. I figured out how to print out all of the ossaudit information however, which helps explain what is failing a little bit more:

# Run this from within the virtualenv of the flask-restx project
ossaudit --column=name --column=version --column=id --column=cve --column=cvss_score --column=title --column=description --installed
+------------+---------+--------------------+----------------+------------+----------------------------------+---------------------------------+
|    name    | version |         id         |      cve       | cvss_score |              title               |           description           |
+============+=========+====================+================+============+==================================+=================================+
| setuptools | 63.3.0  | sonatype-2014-0148 |                | 6.5        | 1 vulnerability found            | 1 non-CVE vulnerability found.  |
|            |         |                    |                |            |                                  | To see more details, please     |
|            |         |                    |                |            |                                  | create a free account at        |
|            |         |                    |                |            |                                  | https://ossindex.sonatype.org/  |
|            |         |                    |                |            |                                  | and request for this            |
|            |         |                    |                |            |                                  | information using your          |
|            |         |                    |                |            |                                  | registered account              |
+------------+---------+--------------------+----------------+------------+----------------------------------+---------------------------------+
| pip        | 22.2.1  | CVE-2018-20225     | CVE-2018-20225 | 7.8        | [CVE-2018-20225] CWE-20:         | ** DISPUTED ** An issue was     |
|            |         |                    |                |            | Improper Input Validation        | discovered in pip (all          |
|            |         |                    |                |            |                                  | versions) because it installs   |
|            |         |                    |                |            |                                  | the version with the highest    |
|            |         |                    |                |            |                                  | version number, even if the     |
|            |         |                    |                |            |                                  | user had intended to obtain a   |
|            |         |                    |                |            |                                  | private package from a private  |
|            |         |                    |                |            |                                  | index. This only affects use of |
|            |         |                    |                |            |                                  | the --extra-index-url option,   |
|            |         |                    |                |            |                                  | and exploitation requires that  |
|            |         |                    |                |            |                                  | the package does not already    |
|            |         |                    |                |            |                                  | exist in the public index (and  |
|            |         |                    |                |            |                                  | thus the attacker can put the   |
|            |         |                    |                |            |                                  | package there with an arbitrary |
|            |         |                    |                |            |                                  | version number). NOTE: it has   |
|            |         |                    |                |            |                                  | been reported that this is      |
|            |         |                    |                |            |                                  | intended functionality and the  |
|            |         |                    |                |            |                                  | user is responsible for using   |
|            |         |                    |                |            |                                  | --extra-index-url securely.     |
+------------+---------+--------------------+----------------+------------+----------------------------------+---------------------------------+
| Flask      | 2.2.0   | sonatype-2020-0201 |                | 4.8        | 1 vulnerability found            | 1 non-CVE vulnerability found.  |
|            |         |                    |                |            |                                  | To see more details, please     |
|            |         |                    |                |            |                                  | create a free account at        |
|            |         |                    |                |            |                                  | https://ossindex.sonatype.org/  |
|            |         |                    |                |            |                                  | and request for this            |
|            |         |                    |                |            |                                  | information using your          |
|            |         |                    |                |            |                                  | registered account              |
+------------+---------+--------------------+----------------+------------+----------------------------------+---------------------------------+
| Sphinx     | 2.1.2   | sonatype-2020-1364 |                | 7.5        | 1 vulnerability found            | 1 non-CVE vulnerability found.  |
|            |         |                    |                |            |                                  | To see more details, please     |
|            |         |                    |                |            |                                  | create a free account at        |
|            |         |                    |                |            |                                  | https://ossindex.sonatype.org/  |
|            |         |                    |                |            |                                  | and request for this            |
|            |         |                    |                |            |                                  | information using your          |
|            |         |                    |                |            |                                  | registered account              |
+------------+---------+--------------------+----------------+------------+----------------------------------+---------------------------------+
| dparse     | 0.5.1   | sonatype-2022-0719 |                | 6.5        | 1 vulnerability found            | 1 non-CVE vulnerability found.  |
|            |         |                    |                |            |                                  | To see more details, please     |
|            |         |                    |                |            |                                  | create a free account at        |
|            |         |                    |                |            |                                  | https://ossindex.sonatype.org/  |
|            |         |                    |                |            |                                  | and request for this            |
|            |         |                    |                |            |                                  | information using your          |
|            |         |                    |                |            |                                  | registered account              |
+------------+---------+--------------------+----------------+------------+----------------------------------+---------------------------------+
Found 5 vulnerabilities in 84 packages

My honest takeaway here is that the ossaudit check should be removed from the project. The one vulnerability that I can actually locate, CVE-2018-20225, doesn't even appear to be a real vulnerability. So while it's good to periodically check for security vulnerabilities, ossaudit is not doing that right now, unfortunately.

@carlosvega
Copy link

carlosvega commented Aug 4, 2022

@plowman can you add them to the list here? like:

# Optional: comman-separated list of vulnerability IDs (Sonatype ID or CVE) to ignore.
ignore-ids = x,y,z

https://github.com/python-restx/flask-restx/blob/master/setup.cfg#L15

@amotl
Copy link

amotl commented Aug 5, 2022

Dear Ben,

we are using flask-restx over at WireViz-Web and appreciate your efforts here very much.

The interim workarounds provided in this discussion and at #460 were really helpful in the meanwhile. Thanks, @j4asper, thanks all.

With kind regards,
Andreas.

@carlosvega
Copy link

Another issue is that ossaudit is not maintained. And the setup.cfg file passed to option --config is not read, as stated in this issue: illikainen/ossaudit#7

I propose to edit the ossaudit test to:

ossaudit --config setup.cfg --installed --ignore-id 06e60262-8241-42ef-8f64-e3d72091de19 --ignore-id sonatype-2022-0719 --ignore-id sonatype-2020-0201 --ignore-id CVE-2018-20225 --ignore-id sonatype-2014-0148 so that they pass since those vulnerabilities are present in the newest versions of the packages and we can't do anything about them for now.

@plowman
Copy link
Contributor Author

plowman commented Aug 11, 2022

Quick update on this PR, I'm still unable to merge until I get the go-ahead from someone with write access:
Screen Shot 2022-08-10 at 5 58 27 PM

Sorry to ping you @j5awry but would appreciate any tips on which maintainer I can talk to in order to get this merged.

To catch you up, here are the current repro steps for this bug:

pip install flask-restx
python -c "import flask_restx"

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/opt/homebrew/lib/python3.9/site-packages/flask_restx/__init__.py", line 5, in <module>
    from .api import Api  # noqa
  File "/opt/homebrew/lib/python3.9/site-packages/flask_restx/api.py", line 50, in <module>
    from .swagger import Swagger
  File "/opt/homebrew/lib/python3.9/site-packages/flask_restx/swagger.py", line 18, in <module>
    from werkzeug.routing import parse_rule
ImportError: cannot import name 'parse_rule' from 'werkzeug.routing' (/opt/homebrew/lib/python3.9/site-packages/werkzeug/routing/__init__.py)

@seanieb
Copy link
Contributor

seanieb commented Aug 11, 2022

@ziirish can you help unblock @plowman?

@jpiwek
Copy link

jpiwek commented Aug 16, 2022

Hi folks,

I am also waiting for a patch:
today I tried with these Py Package versions:
flask==2.2.2
flask_restx=0.5.1
werkzeug==2.2.2
flask-login==0.6.2

It seems like flask_restx.swagger.py makes issues as werkzeug.routing.rules.py has changed. It doesn't support parse_rule function anymore. Instead there is a new private _parse_rule function inside the Rule Class.
I had no time to implement the new Werkzeug Rule API into Flask-RestX. However I created two patch files as an interim solution to get everything running again and to make swagger HTML accessible!
Please let me know if it works for you.
20220815_flask_restx_patches.tar.gz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants